Repository navigation
Detect Spark driver completion by container state when tracking via k8s API - #68048
karenbraganz wants to merge 25 commits into
Conversation
|
I still need to test this out and write unit tests. |
|
This has passed all unit tests as well as a manual test that I ran. |
# Conflicts: # providers/apache/spark/src/airflow/providers/apache/spark/operators/spark_submit.py
aaron-y-chen
left a comment
There was a problem hiding this comment.
Should we also document the changes made in this PR in providers/apache/spark/docs/operators.rst?
|
The original issue described a second problem
This is reachable whenever |
I had initially decided to leave that part of the code as is because it only acts as a fallback when the driver container cannot be identified. However, now I am thinking it might be more accurate to just report the pod failure without the exact container status even in case of the fallback. Something like this: Or we could print the statuses of all containers in the pod without assuming any specific one is the driver. @uranusjr what do you think? I can create a separate PR for this. I have edited this PR description so that it dos not say that the issue is closed. |
|
I think it makes sense to just print the containers’ status. Probably safer and more helpful. This isn’t only a fallback though, it might also be reached when the driver is identified but hasn’t terminated yet. A pod can report phase=Failed while the driver container is still running since a sidecar died first. So maybe
|
| If ``kubernetes_driver_container_name`` doesn't match any container on the pod, the task fails | ||
| immediately with a ``ValueError`` rather than silently falling back to the heuristic. |
There was a problem hiding this comment.
This path also deletes the driver pod, which terminates the running Spark application. That’s a destructive consequence of a typo in a templated field, so it should be stated here rather than only in the exception message.
| """ | ||
| pod_name = self._kubernetes_driver_pod | ||
| kubernetes_driver_pod_name = self._kubernetes_driver_pod_name | ||
| kubernetes_driver_container_name = self.kubernetes_driver_container_name |
There was a problem hiding this comment.
This is a template field, so this can arrive as whitespace from a Jinja render (not uncommon due to how people indent templates). Maybe do a strip(), or only act against None (instead of falsy) instead below?
| phase may not advance to ``Succeeded`` until every container exits. To avoid indefinite | ||
| waits, set ``execution_timeout`` as a hard bound. | ||
|
|
||
| Set ``durable=False`` to always submit a fresh driver on retry. |
There was a problem hiding this comment.
I think this line is stray? (It used to belong to the section above, not the new section)
related: #67934
This PR tracks Spark job completion by container state instead of pod phase when
track_driver_via_k8s_api=True. Sometimes the pod continues to run even after the driver container completes due to other sidecar containers. This PR makes driver completion detection more accurate by examining the container itself.